Skip to content

Prune orphaned CSS artifacts before building - #471

Closed
myabc wants to merge 3 commits into
mainfrom
code-maintenance/prune-orphaned-css
Closed

Prune orphaned CSS artifacts before building#471
myabc wants to merge 3 commits into
mainfrom
code-maintenance/prune-orphaned-css

Conversation

@myabc

@myabc myabc commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What are you trying to accomplish?

script/build-assets left the working tree dirty after a clean build. Renamed or removed components leave behind generated .css files with no .pcss source (e.g. open_project/skeleton_box, open_project/tree_view), and export-css-selectors globs every .css — so the stale selectors got mapped into static/classes.json.

Two complementary changes:

  • Generator (export-css-selectors) — for folders generated from .pcss (app/components/primer), skips any .css without a sibling .pcss, so dead component selectors never reach static/classes.json regardless of tree state or how the script is invoked. Hand-authored stylesheets under app/lib/primer/css have no .pcss and are left untouched via a requireSource flag.
  • Build (build_css) — prunes generated .css/.css.map/.css.json artifacts before regenerating, so orphans don't linger in the tree.

Integration

None — build-tooling change only.

List the issues that this change affects.

Build hygiene fix surfaced while working on TypeScript 6.0 (#470).

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Fixed the correctness bug at its source — the generator now treats the .pcss as the source of truth (.pcss exists ⇔ component exists), so static/classes.json stays correct even if a stale .css reappears or the script runs standalone. The prune is kept as tree hygiene to clear the orphaned artifacts. Both are safe: every .css/.css.map/.css.json under app/components/primer is gitignored and regenerated each build.

Anything you want to highlight for special attention from reviewers?

Build-tooling only — no component or runtime change. Verified: the generator filter alone keeps static/classes.json clean with an orphan present (and still emits the hand-authored app/lib/primer/css json), and a full script/build-assets css leaves the tree clean.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Copilot AI review requested due to automatic review settings June 8, 2026 10:59
@changeset-bot

changeset-bot Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 6ffacb5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@myabc myabc added skip changeset For PRs that contain no relevant release data (e.g. documentation only) maintenance Keeping the fork up do date with the base repo labels Jun 8, 2026
@myabc
myabc requested a review from HDinger June 8, 2026 11:01
Copilot AI reviewed Jun 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures CSS builds start from a clean slate by removing previously generated CSS artifacts that no longer correspond to any .pcss source (e.g., after component renames/removals). This prevents script/export-css-selectors from globbing stale .css files and incorrectly polluting static/classes.json, and avoids leaving the working tree dirty after running the build.

Changes:

  • Add a pre-build prune step in build_css to delete generated .css, .css.map, and .css.json files under app/components/primer before regenerating assets.

@github-actions

github-actions Bot commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ Visual or ARIA snapshot differences found

Our visual and ARIA snapshot tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review differences

@myabc
myabc requested a review from Copilot June 8, 2026 11:16
@myabc myabc added the needs review Needs review label Jun 8, 2026
Copilot AI reviewed Jun 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

Comment thread script/build-assets
Comment on lines +6 to +8
# Remove previously generated CSS artifacts so components that were renamed or
# removed don't leave orphaned files behind. Stale .css would otherwise be
# picked up by export-css-selectors and pollute static/classes.json.
myabc added 2 commits June 20, 2026 21:05
@myabc
Prune orphaned CSS artifacts before building
94294c3 
Renamed or removed components left behind generated .css files with no
.pcss source (e.g. open_project/skeleton_box, open_project/tree_view).
export-css-selectors globbed all .css and mapped the stale selectors
into static/classes.json, leaving the working tree dirty after a build.

Adds a prune step at the start of build_css that deletes the generated
.css/.css.map/.css.json artifacts before regenerating from the current
.pcss sources. All such artifacts are gitignored and regenerable.
@myabc
Skip orphaned CSS without a matching .pcss
88f1e2b 
The prune step removes orphaned .css before a build, but
export-css-selectors itself still mapped any stray .css it globbed
into static/classes.json -- so a leftover file, or running the script
directly, could re-introduce dead component selectors.

Filters the glob to .css that still have a sibling .pcss for folders
generated from .pcss (app/components/primer). Hand-authored stylesheets
under app/lib/primer/css have no .pcss and are left untouched via a
requireSource flag, so their per-file json keeps generating.
@myabc
myabc force-pushed the code-maintenance/prune-orphaned-css branch from 3e80bfd to 88f1e2b Compare June 20, 2026 19:05
@myabc myabc closed this Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@HDinger HDinger Awaiting requested review from HDinger

Assignees

No one assigned

Labels

maintenance Keeping the fork up do date with the base repo needs review Needs review skip changeset For PRs that contain no relevant release data (e.g. documentation only)

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@myabc